- 
                Notifications
    You must be signed in to change notification settings 
- Fork 17
IBX-8138: [Rector] Applied rules from Symfony 5 Rector set lists #385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Merged
      
      
    Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    e3e913f    to
    139b872      
    Compare
  
        
          Base automatically changed from
    
      ibx-8400-fix-redundant-repository-factory
     to
    
      main
    
    June 18, 2024 14:27     
    
876de38    to
    f6c613d      
    Compare
  
    fb6aea3    to
    2cd6d9b      
    Compare
  
    62ca525    to
    9c0b13c      
    Compare
  
    e347340    to
    16e0239      
    Compare
  
    a7f52b2    to
    bb3fa84      
    Compare
  
    bb3fa84    to
    b42a789      
    Compare
  
    b42a789    to
    bcdc124      
    Compare
  
    
              
                    adamwojs
  
              
              approved these changes
              
                  
                    Jul 17, 2024 
                  
              
              
            
            
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done! 🚀
        
          
                src/lib/Persistence/Legacy/Content/FieldValue/Converter/AuthorConverter.php
          
            Show resolved
            Hide resolved
        
      
              
                    konradoboza
  
              
              reviewed
              
                  
                    Jul 17, 2024 
                  
              
              
            
            
        
          
                src/bundle/RepositoryInstaller/Command/ValidatePasswordHashesCommand.php
          
            Show resolved
            Hide resolved
        
              
          
                src/lib/MVC/Symfony/Component/Serializer/AbstractPropertyWhitelistNormalizer.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/lib/MVC/Symfony/Component/Serializer/CompoundMatcherNormalizer.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/lib/MVC/Symfony/Component/Serializer/SimplifiedRequestNormalizer.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
      aae16c6    to
    e2a12d6      
    Compare
  
    
  This was referenced Aug 26, 2024 
      
Applied rules: * LiteralGetToRequestClassConstantRector * ResponseStatusCodeRector * MakeCommandLazyRector
Applied rules: * ReturnTypeFromStrictNativeCallRector * ReturnTypeFromStrictScalarReturnExprRector
Co-authored-by: Konrad Oboza <[email protected]>
e2a12d6    to
    0584976      
    Compare
  
    | 
 | 
    
  barw4 
      pushed a commit
      that referenced
      this pull request
    
      Oct 17, 2024 
    
    
      
  
    
      
    
  
For more details see https://issues.ibexa.co/browse/IBX-8138 and #385 Key changes: * [Rector] Applied Symfony 5.1 CommandConstantReturnCodeRector * [Rector] Applied Symfony 5.2 RenameMethodRector * [Rector] Applied Symfony 5.3 Rector sets * [Rector] Applied Symfony code quality Rector sets Applied rules: * LiteralGetToRequestClassConstantRector * ResponseStatusCodeRector * MakeCommandLazyRector * [Rector] Applied Return type rectors Applied rules: * ReturnTypeFromStrictNativeCallRector * ReturnTypeFromStrictScalarReturnExprRector * [Rector] Applied Symfony 6.0 AddReturnTypeDeclarationRector * [Rector] Added Symfony Bundle::getContainerExtension return type * Added strict types for InstallPlatformCommand consts * Made XML serialization exception more verbose in Author and DateTime converters * Implemented `\Ibexa\Core\MVC\Symfony\Security\User::getUserIdentifier` --------- Co-Authored-By: Paweł Niedzielski <[email protected]> Co-Authored-By: Konrad Oboza <[email protected]>
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    

Caution
Related PRs:
Follow-ups to ibexa/core:
IBX-8138 rector changes for the other packages:
Description:
Applied rules from the Symfony 5 Rector set lists.
This is just one milestone on the way towards Symfony 6. Some Rectors address code quality. Not all deprecations are covered by the Symfony-provided Rectors. Additionally, not all deprecations are related to PHP code, which is out of scope for Rector. Moreover, some deprecations can be resolved via Symfony 6 Rectors, but only some can be applied without upgrading to Symfony 6.
This PR also includes necessary fixes to align changes with the PHPStan baseline and to address return type bugs.
The applied Rectors are as follows:
SF 5.0
AddParamTypeDeclarationRectoradding parameter types for chosen set of Symfony methods, if used by 1st party,SF 5.1
CommandConstantReturnCodeRector- e.g.:return self::SUCCESSinstead of a literalint,SF 5.2
RenameMethodRectorrenaming calls to\Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken::getProviderKeytogetFirewallName(however there's no rector for e.g. deprecatedgetCredentialsmethod of the same class),SF 5.3
RenameMethodRector:RequestStack::getMasterRequest()->RequestStack::getMainRequest(),SF 5.3
RenameClassConstFetchRector:HttpKernelInterface::MASTER_REQUEST->HttpKernelInterface::MAIN_REQUEST,SF 5.3
RenameClassRector:Symfony\Component\Security\Core\Exception\UsernameNotFoundException->Symfony\Component\Security\Core\Exception\UserNotFoundException,SF 5.4
AddReturnTypeDeclarationRector- adding return type?\Symfony\Component\DependencyInjection\Extension\ExtensionInterfacefor classes implementing\Symfony\Component\HttpKernel\Bundle\BundleInterface::getContainerExtension()methodSF 6.0
AddReturnTypeDeclarationRector- adding return types added by Symfony in 6. It was mostly possible to do this on SF 5 for our extension points as those type didn't change, they were just hinted in PHPDoc. Still, required some manual fixes afterwards (see also a few points below - theReturnTypeFrom*Rectorremarks,general code quality rector sets:
MakeCommandLazyRector- replacingsetName()andsetDescription()with$defaultNameand$defaultDescriptionproperties in Commands. Note that this rule however is deprecated, to be replaced with attributes in SF 6.1. For now this is just a "dead middle step" as Symfony itself states in the deprecation message,LiteralGetToRequestClassConstantRectorreplacing'GET'withRequest::GETconstant,ResponseStatusCodeRectorreplacing e.g. 302 withResponse::HTTP_FOUNDconstant in relavant places,ReturnTypeFrom*RectorandAddReturnTypeDeclarationBasedOnParentClassMethodRector- a set of rectors applying return types based on either inferred method body or parent declaration, if feasible - this one has been proved to be quite risky as the returned type didn't always strictly follow what it should return.CI Status
I've spent quite some time trying to rectify code duplication issues. Sadly this is not doable fully in a reasonable time as the entire codebase contains copy-pasted code since 2012. Since the Rector touched many places, all of them pop up now as new code, which makes quality gate fail. I have a set of changes fixing some of those WET code parts, however the scope of this PR is too big. I'll make follow ups.
For QA:
No QA needed. Regression tests should be enough.
Regression run: ibexa/commerce#930
Documentation:
This is code refactoring without behavior change. No documentation changes needed.